Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add simple function to count irreducible polynomials over 𝔽q #36390

Merged

Conversation

yyyyx4
Copy link
Member

@yyyyx4 yyyyx4 commented Oct 3, 2023

There is a well-known formula for counting irreducible polynomials of given degree $n$ over a finite field $\mathbb F_q$, using Möbius inversion. In this patch we add a simple function to evaluate this formula.

@fchapoton
Copy link
Contributor

maybe rather put in src/sage/combinat/q_analogues.py ?

@yyyyx4
Copy link
Member Author

yyyyx4 commented Oct 4, 2023

Pardon my ignorance: Why?

@fchapoton
Copy link
Contributor

because similar polynomials in q (cardinal of finite field) are already there..

and your method should take q as a parameter, and work with q an indeterminate

@maxale
Copy link
Contributor

maxale commented Oct 5, 2023

I have a few of comments:

  1. why not use just one-liner:
return sum( moebius(n//d) * q**d for d in n.divisors() ) // n

instead of the current:

    r = ZZ.zero()
    for d in n.divisors():
        r += moebius(n//d) * q**d
    return r // n
  1. The documentation should state that polynomials are monic.

  2. There exists a formula for the number of monic irreducible multivariate polynomials. It can be seen here or in this paper. I'd suggest to implement it as well, and add the number of variables as yet another function argument with the default value 1.
    You may find my implementation of this formula in PARI/GP at this link

@yyyyx4
Copy link
Member Author

yyyyx4 commented Nov 15, 2023

Thanks for the suggestions: All done, except for the multivariate case, which is left for future work.

@yyyyx4 yyyyx4 force-pushed the public/count_irreducible_polynomials branch from 44235ea to b31a4c5 Compare January 15, 2024 15:37
@GiacomoPope
Copy link
Contributor

Depending on how important the multivariate case is, this could be modified to include m = 1 as an optional parameter with a "NotImplementedError" with comments pointing to the references Max included. Personally I think this can be left for someone else to do though?

One thing I didn't understand was whether the formula in Bodin's work (which is only asymptotic) was exactly the same as Max's comment in the forum, or whether Max's comment and PARI implementation were precise?

Personally I think this univariate one is nice as it is, especially with the polynomial output when q=None and this recursive (potentially only asymptotic) formula for the multivariate one can be solved by someone else when they're interested in doing so?

@maxale
Copy link
Contributor

maxale commented Feb 2, 2024

One thing I didn't understand was whether the formula in Bodin's work (which is only asymptotic) was exactly the same as Max's comment in the forum, or whether Max's comment and PARI implementation were precise?

@GiacomoPope: Bodin's result is not only asymptotic, Lemma 4 in his work gives an exact formula. That formula looks similar to mine (which was posted in the dxdy.ru forum). Historically, my formula appeared first (in 2006), but I considered it minor and did not bother to publish it elsewhere. PARI/GP code implements my formula and it was used to produce terms for many related sequences in the OEIS, some of which (such as A115457) I added back in 2006. I hope this explains the context.

Btw, A115457 links another relevant paper Survey on counting special types of polynomials.

@GiacomoPope
Copy link
Contributor

Thanks for the context Max. This helps. Potentially this function would be useful for people, but I don't think it needs to be included into this PR for this contribution to have value. It can be added at a later date without adding any additional work.

If you disagree though, then maybe porting your PARI code for this PR is something that needs to be done.

@maxale
Copy link
Contributor

maxale commented Feb 3, 2024

I'm not familiar enough with Sage development, and thus I'd appreciate it if someone can properly incorporate my code into this PR. Please find my extension to arbitrary positive m below. In the original code I've changed:

  • q = ZZ['q'].gen() to q = QQ['q'].gen() since in general for m > 1 we may have an integer-valued polynomial but with rational coefficients
  • ZZ(n).divisors() to n.divisors() because conversion n = ZZ(n) is already done as a first line.
  • the case m == 1 unconditionally return r // n since this division is well-defined for polynomials as well.
def number_of_irreducible_polynomials(n, q=None, m=1):
    n = ZZ(n)
    if n <= 0:
        raise ValueError('n must be positive')
    if q is None:
        q = QQ['q'].gen()       # for m > 1, we produce an integer-valued polynomial in q, but it does not necessarily have integer coefficients
    R = parent(q)
    if m == 1:
        from sage.arith.misc import moebius
        r = sum((moebius(n//d) * q**d for d in n.divisors()), R.zero())
        return r // n
    elif m > 1:
        r = []
        for d in range(n):
            r.append( (q**binomial(d+m,m-1) - 1) // (q-1) * q**binomial(d+m,m) - sum(prod(binomial(r_+t-1,t) for r_,t in zip(r,p.to_exp(d))) for p in Partitions(d+1,max_part=d)) )
        return r[-1]
    else:
        raise ValueError('m must be positive')

@yyyyx4
Copy link
Member Author

yyyyx4 commented Feb 5, 2024

Thanks for writing this code @maxale; I've added it to the branch together with the corresponding documentation changes.

Copy link
Contributor

@GiacomoPope GiacomoPope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments, which can probably be taken or left.

src/sage/combinat/q_analogues.py Outdated Show resolved Hide resolved
src/sage/combinat/q_analogues.py Outdated Show resolved Hide resolved
@GiacomoPope
Copy link
Contributor

@yyyyx4 I did something stupid in my suggestion while copy pasting and introduced whitespace issues!

@yyyyx4 yyyyx4 force-pushed the public/count_irreducible_polynomials branch from 8bb63d7 to 0b345e1 Compare February 21, 2024 16:22
maxale and others added 3 commits March 22, 2024 12:14
Co-authored-by: Giacomo Pope <44242839+GiacomoPope@users.noreply.github.com>
@yyyyx4 yyyyx4 force-pushed the public/count_irreducible_polynomials branch from 0b345e1 to d952887 Compare March 22, 2024 11:14
Copy link

Documentation preview for this PR (built with commit d952887; changes) is ready! 🎉

Copy link
Contributor

@mkoeppe mkoeppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, appears to have addressed reviewer comments, passes tests.

@vbraun vbraun merged commit 057bd86 into sagemath:develop Apr 8, 2024
18 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone Apr 8, 2024
@yyyyx4 yyyyx4 deleted the public/count_irreducible_polynomials branch May 15, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants